Refactor Transaction Watcher #386
Conversation
| @@ -24,6 +24,7 @@ interface TokenComputer { | |||
| /** | |||
| * Use this class to create transactions for native token transfers (EGLD) or custom tokens transfers (ESDT/NTF/MetaESDT). | |||
| */ | |||
| // this name is only temporary; the class will be renamed to `TransferTransactionsFactory` | |||
There was a problem hiding this comment.
Adjust comment, to be inside the one above.
| @@ -61,7 +61,7 @@ export class TransactionNextBuilder { | |||
| gasLimit: gasLimit, | |||
| value: this.amount || 0n, | |||
| data: data.valueOf(), | |||
| chainID: this.config.toString(), | |||
| chainID: this.config.chainID, | |||
| @@ -28,6 +28,7 @@ describe("test delegation transactions factory", function () { | |||
| assert.deepEqual(transaction.data, Buffer.from("createNewDelegationContract@010f0cf064dd59200000@0a")); | |||
| assert.equal(transaction.gasLimit, 60126500n); | |||
| assert.equal(transaction.value, value); | |||
| assert.equal(transaction.chainID, config.chainID); | |||
| @@ -58,8 +58,8 @@ describe("fetch transactions from local testnet", function () { | |||
| await provider.sendTransaction(transactionDeploy); | |||
| await provider.sendTransaction(transactionIncrement); | |||
|
|
|||
| await watcher.awaitCompleted(transactionDeploy); | |||
| await watcher.awaitCompleted(transactionIncrement); | |||
| await watcher.awaitCompleted(transactionDeploy.getHash().hex()); | |||
There was a problem hiding this comment.
In a future PR, we should also adjust the integration tests to use the transaction factories for deploy, call etc.
src/transaction.local.net.spec.ts
Outdated
| it("should create transaction using the NextTokenTransferFactory", async function () { | ||
| this.timeout(70000); | ||
|
|
||
| let provider = createLocalnetProvider(); |
There was a problem hiding this comment.
Here & below, use const when applicable.
| @@ -325,4 +330,51 @@ describe("test on local testnet", function () { | |||
| queryResponse = await provider.queryContract(query); | |||
| assert.equal(decodeUnsignedNumber(queryResponse.getReturnDataParts()[0]), 0); | |||
| }); | |||
|
|
|||
| it("counter: should deploy and call using the SmartContractFactory", async function () { | |||
There was a problem hiding this comment.
🚀 👍 (in relation to the other comment about a next PR for integration tests, only now I see this test)
| await alice.sync(provider); | ||
|
|
||
| const transactionComputer = new TransactionComputer(); | ||
| const config = new TransactionsFactoryConfig(network.ChainID); |
There was a problem hiding this comment.
Now I see - we should pass init parameters to TransactionsFactoryConfig as options (even if now there's only one, the chain ID) - similar to SmartContractTransactionsFactory etc. This, in order to avoid breaking changes in the future, if the config should be extended to receive an extra parameter in the constructor.
This would be for another, separate PR.
There was a problem hiding this comment.
Will do in the next PR.
| bytecode: bytecode, | ||
| gasLimit: 3000000n, | ||
| }); | ||
| deployTransaction.nonce = BigInt(alice.account.nonce.valueOf()); |
There was a problem hiding this comment.
A bit verbose, but good at this moment. In the cookbook I think we will not mention AccountNonce class anymore - left for the client code to hold & handle the nonce.
popenta
changed the title
|
The breaking part was recovered in #404. |
andreibancioiu
changed the title
Later edit: the breaking part was recovered in #404.
This PR introduces breaking changes.Before, the methods in the
TransactionWatcherclass required an input parameter of typeITransaction. Now, in order to be compatible with theTransactionNextclass, the methods simply require the transaction hash.Migrated from:
to: